feat: provided-style extensions and manifest-libs#791
feat: provided-style extensions and manifest-libs#791
Conversation
There was a problem hiding this comment.
It looks like the pull request diff is too large to be displayed directly on GitHub. Here are some general recommendations for handling large pull requests:
-
Break Down the PR: Try to break down the pull request into smaller, more manageable chunks. This makes it easier to review and ensures that each part can be thoroughly checked.
-
Review Locally: Clone the repository and check out the branch locally. This allows you to review the changes using your preferred tools and environment.
-
Focus on Key Areas: Identify and focus on the key areas of the code that are most critical or have the highest impact. This can help prioritize the review process.
-
Automated Tools: Use automated code review tools and linters to catch common issues and enforce coding standards.
-
Collaborate with the Team: If possible, involve multiple reviewers to share the workload and get different perspectives on the changes.
If you can provide a smaller subset of the changes or specific files that need review, I'd be happy to help with a more detailed review.
f5dd242 to
0062717
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces provided-style extensions and manifest-driven library loading to enable tracing application-specific classes without polluting the application classpath. The core innovation is using reflection and object hand-off patterns to access application classes at runtime, keeping BTrace isolated.
Changes:
- New extension utilities (
ClassLoadingUtil,MethodHandleCache) for runtime class resolution and reflective method access - Manifest-driven library path resolution (
AgentManifestLibs) with security restrictions to BTRACE_HOME - Refactored agent classpath processing with deduplication and better error handling
- Example extensions for Spark and Hadoop demonstrating the provided-style pattern
- Comprehensive architecture documentation and migration guides
- Deprecation of
libs/profiles mechanism with escape hatch for edge cases
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
settings.gradle |
Added example extension modules (btrace-spark, btrace-hadoop) |
integration-tests/src/test/java/tests/RuntimeTest.java |
Fixed debug flag insertion and added timing delays for test stability |
integration-tests/src/test/java/tests/ManifestLibsTests.java |
New test suite for manifest-libs feature |
integration-tests/src/test/java/tests/BTraceFunctionalTests.java |
Improved unattended test with better synchronization and cleanup |
integration-tests/build.gradle |
Added test property for skipping preconfigured libs |
docs/examples/README.md |
Guide for building and configuring provided-style extension examples |
docs/architecture/provided-style-extensions.md |
Comprehensive guide on provided-style extension pattern |
docs/architecture/migrating-from-libs-profiles.md |
Migration guide from deprecated libs/profiles to extensions |
docs/architecture/agent-manifest-libs.md |
Design document for manifest-driven library loading |
compile.log |
Build artifact that should not be committed |
btrace-extensions/examples/btrace-spark/* |
Example Spark extension with API and provided-style implementation |
btrace-extensions/examples/btrace-hadoop/* |
Example Hadoop extension with API and provided-style implementation |
btrace-extension/src/main/java/org/openjdk/btrace/extension/util/MethodHandleCache.java |
Cache for reflective method handles to reduce lookup overhead |
btrace-extension/src/main/java/org/openjdk/btrace/extension/util/ClassLoadingUtil.java |
Helper utilities for TCCL-based class loading and service discovery |
btrace-client/src/main/java/org/openjdk/btrace/client/Main.java |
Removed exit hooks for list-only operations to prevent unintended probe removal |
btrace-client/src/main/java/org/openjdk/btrace/client/Client.java |
Added agent-already-running detection and system property pass-through |
btrace-agent/src/main/java/org/openjdk/btrace/agent/RemoteClient.java |
Improved disconnect handling with disconnecting flag |
btrace-agent/src/main/java/org/openjdk/btrace/agent/Main.java |
Refactored classpath processing with manifest support, deduplication, and escape hatch for single-jar injection |
btrace-agent/src/main/java/org/openjdk/btrace/agent/AgentManifestLibs.java |
New utility for manifest-driven library resolution with security checks |
README.md |
Added deprecation notice for libs/profiles with migration guidance |
|
@jbachorik I've opened a new pull request, #792, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@jbachorik I've opened a new pull request, #793, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@jbachorik I've opened a new pull request, #794, to work on those changes. Once the pull request is ready, I'll request review from you. |
ReDoS in
|
b2aab6c to
75229c8
Compare
…ibs deprecation; provided-style helpers; examples; tests\n\n- agent: manifest-driven libs (flagged), centralized classpath append with dedup + diagnostics; deprecate libs (debug log); add single-jar system CL escape hatch; test knob scoped to manifest-libs\n- client: pass-through of btrace.* props to agent using $ system props\n- extension: add ClassLoadingUtil and MethodHandleCache for provided-style extensions\n- examples: add btrace-extensions/examples/{btrace-spark,btrace-hadoop} with README\n- docs: research + provided-style guide + migration + examples index; README deprecation note\n- tests: add ManifestLibsTests; gradle knob for btrace.test.skipLibs\n\nBREAKING CHANGE: libs/profiles deprecated with planned removal after N+2; prefer extensions
- Proactively retransform java.lang.Thread after startup scripts load - Move initExtensions() after transformer is installed - Add delay in RuntimeTest to allow traced app output after ready marker 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ation - Add disconnecting flag to RemoteClient for immediate probe visibility - Skip loadAgent when agent already running (check btrace.port) - Add try-finally cleanup in tests to prevent port conflicts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…il/ClassLoadingUtil.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…cation Add RunnableResult + compileAndLoad() to CompileTestHarness so compiled classes can be loaded and invoked; add generatedAdapterInvokesRealMethod test that exercises the full findVirtual + MethodHandle.invoke pipeline. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… api source set Wire btrace-extension-processor as an automatic apiAnnotationProcessor dependency in BTraceExtensionPlugin so extension authors get adapter generation for free. In-tree builds use the sibling project reference; external consumers resolve the published Maven artifact by version. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…atestSupported Apply Types#erasure() to return and parameter types in buildSpec() so that parameterized types like List<String> produce raw-type literals (List.class) in generated adapter source instead of the invalid List<String>.class form. Replace @SupportedSourceVersion(RELEASE_8) with getSupportedSourceVersion() returning SourceVersion.latestSupported() to eliminate the javac warning on JDK 11+. Adds a test covering both parameterized-type cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…xternalType - Add btrace-ext-test module: lightweight extension with ExternalDataType (@ExternalType interface) and ExternalTypeTestService to exercise the full annotation-processor → adapter → BTrace probe pipeline - Add integration test (ExternalTypeAdapterIntegrationTest) verifying that a BTrace probe annotated with @injected can call an extension service backed by a generated $Ext adapter against a class present only in the target app's classloader - Guard TCCL null in generated static-dispatch adapter (fallback to ClassLoader.getSystemClassLoader()) so adapters work on bootstrap threads - Move buildSpec() call inside the try/catch in ExternalTypeProcessor so processor-internal errors are reported as compile errors, not crashes - Add TCCL unit tests: staticDispatcherResolvesViaContextClassLoader and staticDispatcherFallsBackToSystemLoaderWhenTcclNull - Simplify AdapterEmitter: single-pass paramAndArgLists() replaces two separate passes; extract shared formatErrors() helper in CompileTestHarness; pass externalFqn into buildSpec() to avoid re-fetching the annotation - Add explanatory comment to empty catch in SparkApiImpl example Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@copilot apply changes based on the comments in this thread |
…arations, broken link Agent-Logs-Url: https://github.com/btraceio/btrace/sessions/697859ef-3024-46cc-915b-7638aaafca8d Co-authored-by: jbachorik <738413+jbachorik@users.noreply.github.com>
Applied all changes from the review thread in commit
|
| > **Note:** Examples use `btrace.jar` -- the single masked JAR (BTrace 2.2+). If using a legacy multi-JAR distribution, replace `btrace.jar` with `btrace-agent.jar` (and add `-Xbootclasspath/a:btrace-boot.jar` where needed). | ||
|
|
|
|
||
| Or via agent: | ||
| ```bash | ||
| java -javaagent:btrace-agent.jar=script=MyTrace.class,filterValue=custom MyApp | ||
| java -javaagent:btrace.jar=script=MyTrace.class,filterValue=custom MyApp | ||
| ``` | ||
|
|
| class BTraceFatAgentPluginTest { | ||
|
|
||
| @TempDir | ||
| Path projectDir; | ||
|
|
||
| private File buildFile; | ||
| private File settingsFile; | ||
|
|
||
| @BeforeEach | ||
| void setUp() throws IOException { | ||
| buildFile = projectDir.resolve("build.gradle").toFile(); | ||
| settingsFile = projectDir.resolve("settings.gradle").toFile(); | ||
|
|
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/btraceio/btrace/sessions/af8301af-e423-407c-90ef-6d91eda3a593 Co-authored-by: jbachorik <738413+jbachorik@users.noreply.github.com>
Applied all changes from the review thread in commit
|
…ations - NestedJarExtensionClassLoader: track bootstrap JarFile as a field and close it in close() to prevent fd leak; replace manual buffer copy loop with Files.copy - MethodHandleCache: use putIfAbsent return value to avoid a third map lookup on concurrent-miss path - AgentManifestLibs: call locateAgentPath once (was called twice per resolve); hoist home.toRealPath() out of the per-entry loop; remove redundant 'Package-private for testing' comments; change readManifest to accept Path directly - BTraceFatAgentPlugin: delete dead resolveExtensions wrapper method - PermissionScanner: delete dead scanClass method and its two unused imports Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| // Include exported extension APIs explicitly without depending on internal source-set names. | ||
| def utilsApi = files(project(':btrace-extensions:btrace-utils').tasks.named('buildApiJar')) | ||
| def statsdApi = files(project(':btrace-extensions:btrace-statsd').tasks.named('buildApiJar')) | ||
| def fullCp = rtCp.plus(extraCp).plus(utilsApi).plus(statsdApi) |
Rationale
BTrace has historically relied on
libs=<profile>and$BTRACE_HOME/btrace-libs/<profile>/{boot,system}/*.jarto make application APIs (Spark, Hadoop, Kafka, …) visible to probes. That model has three structural problems:$BTRACE_HOMEwith a matching profile layout. Users end up with custom init containers, sidecars, or wrapper scripts just to deliver one extra JAR.This PR introduces three complementary mechanisms to fix this: provided-style extensions (APIs on bootstrap, implementation reflective and isolated),
@ExternalTypeadapter generation (build-time boilerplate elimination), and fat agents (a single JAR containing the agent plus embedded extensions, built via a Gradle or Maven plugin).Usability Improvements
java -javaagent:btrace-agent-fat.jaris now enough for Spark/Hadoop/K8s — no$BTRACE_HOME, no profile directory, no sidecars.@ExternalTypeannotation processor. Annotate an interface with@ExternalType("fully.qualified.AppType")and the build generates a companion$Extclass with typed, lazily-resolved static dispatchers per method. No manualMethodHandleCache/ClassLoadingUtilboilerplate for the common case.org.openjdk.btrace.extension,org.openjdk.btrace.fat-agent) and abtrace-maven-pluginhandle API/impl split, manifest metadata, permission scanning, relocations, multi-project auto-discovery, and annotation processor registration.api,impl,main) rather than separate projects, which simplifies build setup.libs=heuristic is replaced by explicit manifest attributes (BTrace-Boot-Libs,BTrace-System-Libs,BTrace-Embedded-Extensions) with deduplication, validation, and diagnostics.btrace-extensions/examples/btrace-sparkandbtrace-extensions/examples/btrace-hadoopdemonstrate the pattern end-to-end, including@ExternalType-based adapters.Quick Start
1. Build a fat agent with Gradle
plugins { id 'org.openjdk.btrace.fat-agent' version "${btraceVersion}" } btraceFatAgent { baseName = 'my-btrace-agent' embedExtensions { project(':my-spark-extension') // multi-project maven('io.btrace:btrace-metrics:2.3.0') // external } }./gradlew fatAgentJar # -> build/libs/my-btrace-agent.jar2. Attach it
Embedded extensions are discovered automatically from the JAR manifest — no
$BTRACE_HOMErequired.3. Write a provided-style extension with
@ExternalTypeDeclare the external type contract in the API source set — the annotation processor (auto-registered by the Gradle plugin) generates
SparkListenerJobStartType$Ext:Use the generated class directly in the impl — no manual
MethodHandleorClassLoadingUtilcode needed:The generated dispatcher resolves the external class lazily on the first call (via the object's defining class loader), caches the
MethodHandle, and retries on subsequent calls if the class wasn't loaded yet — noExceptionInInitializerErrorat extension load time.For cases
@ExternalTypedoesn't cover (field access, constructors, non-public visibility), the manualClassLoadingUtil/MethodHandleCachehelpers still work alongside generated adapters.4. (Optional) Static external methods
Annotate an interface method with
@ExternalType.Static— the processor generates afindStatic+ TCCL-based dispatcher:Maven equivalent
See
docs/architecture/provided-style-extensions.mdanddocs/architecture/migrating-from-libs-profiles.mdfor the full walk-through.Drawbacks and Remaining Issues
@ExternalTypev1 scope limits. The annotation processor does not cover field access (read/write), constructors,instanceof/checkcaston external types, chained@ExternalTypereferences, or non-publicmethods. UseClassLoadingUtil/MethodHandleCachedirectly for these.$BTRACE_HOMEdon't benefit from embedding.@ExternalTypecases. When@ExternalTypedoesn't cover your case, the provided-style pattern still requires explicit API/impl split and reflective adapters. It's a deliberate trade-off (isolation + portability), but the migration cost from existing profile-based setups is real.MethodHandles (generated or manual), an adapter call is more expensive than a direct invocation. Very high-frequency probes should be profiled.libs=/profiles=still accepted. They are deprecated and log a warning; planned removal is N+2 minor releases. Until then, both code paths coexist and users can mix them (not recommended).-Dbtrace.system.appendJar=/abs/path/lib.jar(requires-Dbtrace.trusted=true) exists for cases where immediate migration is impossible. It's restricted toBTRACE_HOMEby default and is explicitly discouraged — expect it to be removed together withlibs=.Documentation
@ExternalTypeprocessor@ExternalTypeand API interfacesBreaking Changes
libs=andprofiles=are deprecated. Planned removal: N+2 minor releases. A runtime warning is emitted on use.🤖 Generated with Claude Code
This change is